Conversation
🦋 Changeset detectedLatest commit: 76f237b The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
View your CI Pipeline Execution ↗ for commit 76f237b
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/devtools
@tanstack/devtools-a11y
@tanstack/devtools-client
@tanstack/devtools-ui
@tanstack/devtools-utils
@tanstack/devtools-vite
@tanstack/devtools-event-bus
@tanstack/devtools-event-client
@tanstack/preact-devtools
@tanstack/react-devtools
@tanstack/solid-devtools
@tanstack/vue-devtools
commit: |
harry-whorlow
force-pushed
the
extract-theme-provider
branch
from
cddbb15 to
2c03299
Compare
harry-whorlow
force-pushed
the
extract-theme-provider
branch
from
b0fb67b to
8048d9d
Compare
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughRenames the exported Theme type to TanStackDevtoolsTheme, re-exports it from devtools-ui, updates consuming types across packages, removes the ThemeProvider wrapper from the lazy mount implementation, and adds a changeset bumping package versions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/devtools-utils/src/solid/class.ts (1)
26-26: Consider usingThemeTypefor consistency.The
mountmethod uses the literal type'light' | 'dark'whileDevtoolPropsusesThemeType. For consistency and maintainability, consider usingThemeTypehere as well.♻️ Suggested change
- async mount<T extends HTMLElement>(el: T, theme: 'light' | 'dark') { + async mount<T extends HTMLElement>(el: T, theme: ThemeType) {And similarly on line 68:
- async mount<T extends HTMLElement>(_el: T, _theme: 'light' | 'dark') {} + async mount<T extends HTMLElement>(_el: T, _theme: ThemeType) {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools-utils/src/solid/class.ts` at line 26, The mount method and its other occurrences use the literal type 'light' | 'dark' instead of the shared ThemeType; update the signature of async mount<T extends HTMLElement>(el: T, theme: 'light' | 'dark') to use ThemeType, and replace any other literal occurrences (e.g., the usage around line 68) with ThemeType so the code consistently imports and references the same ThemeType used by DevtoolProps (ensure ThemeType is imported where needed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/cyan-times-marry.md:
- Line 6: Typo in the changeset description: locate the changeset entry whose
summary reads "Extract devtools-ui from devtools-utils to avoid theme
miss-match" and correct the word "miss-match" to "mismatch" so the summary reads
"Extract devtools-ui from devtools-utils to avoid theme mismatch".
---
Nitpick comments:
In `@packages/devtools-utils/src/solid/class.ts`:
- Line 26: The mount method and its other occurrences use the literal type
'light' | 'dark' instead of the shared ThemeType; update the signature of async
mount<T extends HTMLElement>(el: T, theme: 'light' | 'dark') to use ThemeType,
and replace any other literal occurrences (e.g., the usage around line 68) with
ThemeType so the code consistently imports and references the same ThemeType
used by DevtoolProps (ensure ThemeType is imported where needed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f526506c-03ef-4c24-94d6-2fabe60212f9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.changeset/cyan-times-marry.mdpackages/devtools-ui/src/components/theme.tsxpackages/devtools-ui/src/index.tspackages/devtools-ui/src/styles/use-styles.tspackages/devtools-utils/bin/intent.jspackages/devtools-utils/package.jsonpackages/devtools-utils/src/solid/class-mount-impl.tsxpackages/devtools-utils/src/solid/class.ts
| '@tanstack/devtools-ui': patch | ||
| --- | ||
|
|
||
| Extract devtools-ui from devtools-utils to avoid theme miss-match |
There was a problem hiding this comment.
Typo in changeset description.
"miss-match" should be "mismatch".
-Extract devtools-ui from devtools-utils to avoid theme miss-match
+Extract devtools-ui from devtools-utils to avoid theme mismatch📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Extract devtools-ui from devtools-utils to avoid theme miss-match | |
| Extract devtools-ui from devtools-utils to avoid theme mismatch |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.changeset/cyan-times-marry.md at line 6, Typo in the changeset description:
locate the changeset entry whose summary reads "Extract devtools-ui from
devtools-utils to avoid theme miss-match" and correct the word "miss-match" to
"mismatch" so the summary reads "Extract devtools-ui from devtools-utils to
avoid theme mismatch".
harry-whorlow
force-pushed
the
extract-theme-provider
branch
from
cd63494 to
00179c0
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/devtools-ui/src/components/icons.tsx (1)
44-44: Consider removing commented-out icon implementations.The file contains several large blocks of commented-out code (lines 44-244, 471-491, 740-888, 889-1755). These appear to be older or alternate implementations of icons that have active versions elsewhere in the file (e.g.,
Listat line 294,ChevronDownIconat line 572).If these are no longer needed, consider removing them to reduce file size and improve maintainability. If they're kept intentionally for reference, a brief comment explaining why would help future maintainers.
Also applies to: 471-471, 740-740, 889-889
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools-ui/src/components/icons.tsx` at line 44, This file contains large blocks of commented-out icon implementations that duplicate active icons (e.g., List and ChevronDownIcon) and bloat the file; remove the unused commented-out icon blocks (the large comment ranges around lines containing List and ChevronDownIcon and other repeats) or, if you intentionally want to keep them, replace each block with a single short explanatory comment stating why the old implementation is retained for reference, and ensure any retained comments reference the active symbol names (e.g., List, ChevronDownIcon) so future maintainers understand the reason.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/devtools-ui/src/components/icons.tsx`:
- Line 44: This file contains large blocks of commented-out icon implementations
that duplicate active icons (e.g., List and ChevronDownIcon) and bloat the file;
remove the unused commented-out icon blocks (the large comment ranges around
lines containing List and ChevronDownIcon and other repeats) or, if you
intentionally want to keep them, replace each block with a single short
explanatory comment stating why the old implementation is retained for
reference, and ensure any retained comments reference the active symbol names
(e.g., List, ChevronDownIcon) so future maintainers understand the reason.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed907713-1e8c-4d6c-bf4c-f523acbfdbf8
📒 Files selected for processing (8)
examples/solid/devtools-ui/src/index.tsxpackages/devtools-a11y/src/core/styles/styles.tspackages/devtools-ui/src/components/icons.tsxpackages/devtools-utils/package.jsonpackages/devtools/src/index.tspackages/preact-devtools/src/devtools.tsxpackages/react-devtools/src/devtools.tsxpackages/solid-devtools/src/core.tsx
💤 Files with no reviewable changes (1)
- packages/devtools-utils/package.json
1 participant
Moves theme provider out of utils. Currently a mismatch of devtools and devtools utils will result in breaking an application. Relocating the theme provider to the respective devtools will stop these errors.
Summary by CodeRabbit
Chores
Refactor
New Features